Improve Redis stream reclaim and prefetch handling - #129
Conversation
13d4bf1 to
2503827
Compare
RedisStreamBroker reclaim was driven by a fixed idle_timeout and a broker-side Redis lock, which both double-ran long tasks and recovered short crashed tasks slowly. Replace it with per-task-deadline reclaim: - Resolve reclaim deadline from the message's `timeout` label (via formatter.loads) plus reclaim_timeout_grace, falling back to idle_timeout for messages without a timeout label. - Drop the autoclaim Redis lock; rely on XCLAIM min-idle-time for server-side atomic dedup (unacknowledged_lock_timeout is deprecated and ignored). - Protect only messages held by the current listener instance from reclaim (tracked via a local delivered set), so a worker sharing a consumer_name with a dead predecessor still recovers its pending. - Gate reclaim sweeps with reclaim_interval (default 30s) to avoid scanning pending on every listen iteration. - Enforce prefetch backpressure: do not XREADGROUP while xread_count delivered-but-unacked messages are outstanding. - Claim broker-local buffered entries to an internal abandoned consumer on listener close, so the next reclaim sweep can recover not-yet-yielded messages immediately. - Recreate a missing consumer group on NOGROUP during xpending/xreadgroup. Tests cover timeout-label reclaim, idle_timeout reclaim, shared consumer_name reclaim, prefetch backpressure, buffered-message handoff on listener close, and NOGROUP self-heal. README documents the new reclaim/backpressure/close behavior.
2503827 to
ee60f08
Compare
|
@s3rius I would appreciate your feedback on the direction of this PR. It improves
The design was partly informed by |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #129 +/- ##
==========================================
+ Coverage 91.70% 94.43% +2.73%
==========================================
Files 7 8 +1
Lines 434 845 +411
==========================================
+ Hits 398 798 +400
- Misses 36 47 +11 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Since opening this PR, I continued the Redis Streams-specific work in a standalone package: https://github.com/vvanglro/taskiq-redis-streams That package is now the reference implementation for the direction I would like to explore. Its main design difference is liveness-based recovery: each generated consumer keeps a TTL heartbeat, and pending entries are reclaimed only after the previous owner's heartbeat expires. This means a live worker can run a task for an arbitrary duration without using Taskiq's It also uses a dedicated heartbeat connection, separates read batch size from outstanding-message capacity ( This intentionally differs from #129's compatibility-oriented design: per-task timeout reclaim with I do not plan to keep expanding #129 into a full replacement without agreement on that direction. Would you prefer a smaller, compatibility-preserving upstream PR for selected improvements, or is the standalone broker a better place to continue this stream-specific design? |
|
I'd love to read your implementation first to understand the magnitude of incompatibility. And I would also want to fully understand how it works. Sorry for the delay. I'll try to make it this week. |
Summary
This PR improves
RedisStreamBrokerreliability and fairness by adopting stronger Redis Streams handling patterns: per-task-deadline reclaim, lock-freeXCLAIM, listener-local prefetch backpressure, and faster handoff of buffered messages on listener close.What changed
XCLAIMrecovery.timeoutlabel plusreclaim_timeout_grace, falling back toidle_timeoutfor messages without a timeout label.unacknowledged_lock_timeout; RedisXCLAIMmin_idle_timenow provides the atomic claim guard.reclaim_intervalso pending-message scans are throttled instead of running on every listen loop.xread_countcaps delivered-but-unacknowledged messages for a single-stream listener.additional_streams. It remains supported for compatibility, but will be removed in a future major release; use one broker and worker process per stream instead.consumer_namecan still recover pending messages from its predecessor.abandonedconsumer and stamp them as very idle, so the next reclaim sweep can recover them immediately.Why
The previous reclaim path used a single global
idle_timeoutplus a broker-side Redis lock. That can reclaim long tasks too early (for example, a task with a 30m timeout reclaimed at the default 10midle_timeout) and recover short crashed tasks slowly (for example, a 5s task waiting for the default 10midle_timeout). The lock also adds a failure point (unacknowledged_lock_timeout) and extra round-trips, while RedisXCLAIMmin_idle_timealready deduplicates claims atomically on the server.additional_streamsmakes strict listener-level prefetch limits ambiguous: Redis appliesXREADGROUP COUNTper stream, not across all streams in a multi-stream read. One broker per stream keeps consumer ownership, reclaim behavior, and prefetch capacity independent.The close-time handoff is deliberately limited to the broker's internal buffer: messages already yielded to taskiq may be executing, so they are not abandoned immediately to avoid duplicate execution during shutdown.
Behavior notes
XREADGROUPwithin a listen iteration, so overdue pending messages take priority over new messages when a sweep is due.reclaim_intervaldefaults to 30000 ms; set it to0to scan on every iteration.consumer_name) are recoverable.xread_countlimits delivered-but-unacknowledged messages for a single-stream broker.xread_count=Nonedisables the limit.additional_streamsemitsDeprecationWarningwhen configured.abandonedconsumer on listener close; already-yielded entries still follow their normal ack/reclaim lifecycle.Tests
Added coverage for:
consumer_namexread_countbackpressureadditional_streamsdeprecation warningValidation run locally:
uv run ruff check taskiq_redis/redis_broker.py tests/test_broker.py uv run mypy taskiq_redis/redis_broker.py tests/test_broker.py uv run pytest tests/test_broker.py -k "not cluster and not sentinel" -qResult:
16 passed, 7 deselectedfor the single-node Redis broker tests.Reference
The reclaim/backpressure design was informed by the Redis Streams broker implementation in
sylvinus/dramatiq-redis-streams, especially its use of deadline-aware pending recovery, bounded local prefetching, and abandoned-buffer handoff on close.